FileSink: keep the event loop alive until end() has drained a pipe - #41709
Conversation
A chunk larger than the pipe buffer leaves its tail in the sink's buffer. end() after that takes a short flush and sets done. The deferred auto-flush task then released the writable poll's ref on the loop because done was set, and the process exited with the tail unwritten. Release the ref only when the buffer is empty. When done is set with bytes still buffered, the writable poll drains them and holds the loop until then.
|
Warning Review limit reached
On-demand reviews are free for the next 14 days. After that, they cost $0.25 per reviewed file. Or wait 2 minutes for your next included review. View limit detailsLimit details: You’ve used all 10 included reviews currently available. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (2)
Comment |
|
Updated 11:46 AM PT - Sep 6th, 2026
⏳ @robobun, your commit ce4578f is still building in
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
…or the flag On Windows uv_write takes the whole chunk, so end() returns a number and .then() threw. The stdin test's reader now gives up after a deadline, and the parent writes the flag from a finally block, so the reader cannot be orphaned by a parent that threw.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Status Reproduced on 1.4.3 (Linux x64) with CI, build #111650:
The diff is ready for review. |
2 participants
Problem
FileSink(Bun.stdout.writer(),Bun.file(fifo).writer(),Bun.spawnstdin) loses data whenwrite()goes async and the script does not awaitend().bun a.mjs | (sleep 1; wc -c)with a 4 MiB chunk delivers 65536 bytes. The process exits with code 0 a few ms later. Anawait end()inside an async function (not top level) loses the data the same way.on_auto_flush(src/runtime/webcore/FileSink.rs:855).write()enabled the writable poll's ref on the loop for the buffered tail.end()took a short flush, setdone, and returned the pending promise. The deferred auto-flush task then sawdoneand calledupdate_ref(false). Nothing held the loop, so it exited before the poll could drain the tail.Fix
on_auto_flushreleases the poll's ref only when the buffer is empty. Whendoneis set with bytes still buffered, it unregisters itself and leaves the ref in place. The writable poll drains the tail.on_writereleases the ref once the buffer is empty, thenend_writercloses the sink and settles the promise.write()withoutend()already kept the loop alive for the same bytes. Node keeps the process alive for a pendingstdout.writetoo.test/js/bun/util/filesink.test.ts(three new tests, all fail on 1.4.3: 694336 and 438528 of 4194304 bytes, and an empty count from the unref'd child). Also the rest of that file,bun-write.test.js, the spawn stdin suites,spawn.test.ts,child_process.test.ts,process-stdio.test.ts.Background
FileSinkis the native writer behindBlob.writer()and a subprocessstdinpipe. It owns aStreamingWriterwith a byte buffer and, on a pipe or socket, aFilePollregistered for writability.FilePoll::enable_keeping_process_alivebumps the uws loop'sactivecount. The loop exits when that count and the task queues are all zero.update_ref(bool)on the sink turns this on or off.on_writeruns after each drain. It sets the ref fromhas_pending_data(). TheAutoFlusherrunson_auto_flushas a deferred task after the current tick, to flush small buffered writes without a poll wake.Notes
write()alone, with noend(), delivers all 4 MiB on 1.4.3. Only theend()path lost data, which pointed at thedonecheck.Bun.file(fifo).writer()) delivered 3489792 of 4194304 on 1.4.3 and 4194304 with the fix.Bun.spawnstdin withunref(): on 1.4.3 the count depends on how fast the child reads. Awc -cthat reads from the start often gets all 4 MiB because the first synchronous write loop keeps up. The test makes the child wait untilend()was called before it reads, so the pipe is full and the result is stable.on_auto_flush. Therehas_pending_data()also covers auv_writein flight, so the ref now stays until the write callback runs.bun-write.test.js"copyFileRange is not available > on large files" (5 s timeout in debug),process-stdio.test.tsthe threeprocess.stdintests,child_process.test.ts"spawn in the default shell" and "extra stdio pipes are not double-closed on GC".no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/util/filesink.test.ts